Update rrfs-test/CMakeLists.txt to be more compact for adding a CTest #81

SamuelDegelia-NOAA · 2024-06-14T16:39:42Z

This PR updates RDASApp/rrfs-test/CMakeLists.txt to be a little more user-friendly by making it easier to add CTests. Two new functions are added to allow for a “dictionary-like” data structure in CMake. To add a new CTest now, you just need to add a single line to either the fv3-jedi or mpas-jedi lists at the top of the file:

### Define CTests here ###

# FV3-JEDI tests
set(rrfs_fv3jedi_tests "")
add_to_dictionary(rrfs_fv3jedi_tests "rrfs_fv3jedi_hyb_2022052619"     "fv3jedi_var.x")
add_to_dictionary(rrfs_fv3jedi_tests "rrfs_fv3jedi_letkf_2022052619"   "fv3jedi_letkf.x")

# MPAS-JEDI tests
set(rrfs_mpasjedi_tests "")
add_to_dictionary(rrfs_mpasjedi_tests "rrfs_mpasjedi_2022052619_Ens3Dvar"     "mpasjedi_variational.x")
add_to_dictionary(rrfs_mpasjedi_tests "rrfs_mpasjedi_2022052619_atms_npp_qc"  "mpasjedi_variational.x")
add_to_dictionary(rrfs_mpasjedi_tests "rrfs_mpasjedi_2022052619_letkf"        "mpasjedi_enkf.x")

All CTests in RDASApp/build/rrfs-test pass after making this change.

Also, per discussion in #79, we will likely move to one mega executable in the near future (i.e., just a single rdas_jedi.x instead of fv3jedi_var.x, fv3jedi_letkf.x, mpasjedi_enkf.x, etc.). CMakeLists.txt will need to be updated again at that point, but we should be able to keep the same new code structure that is added in this PR. It will eventually look something like:

add_to_dictionary(rrfs_tests “rrfs_fv3jedi_hyb_2022052619” “fv3jedi” “variational”)

rrfs-test/CMakeLists.txt

guoqing-noaa · 2024-06-14T21:09:33Z

I would think we probably do "over-engineering" here.
@SamuelDegelia-NOAA 's first solution in the issue #79 without a function is enough to handle the problem and is easy to read by any developers. More importantly, if we decide to use one single executable, this change will be replaced soon. I would think investing efforts in one single executable will be more valuable. My two cents.

TingLei-NOAA · 2024-06-14T21:31:56Z

I would think we probably do "over-engineering" here. @SamuelDegelia-NOAA 's first solution in the issue #79 without a function is enough to handle the problem and is easy to read by any developers. More importantly, if we decide to use one single executable, this change will be replaced soon. I would think investing efforts in one single executable will be more valuable. My two cents.

I am really wondering when there would be a super executable to be realized ,and I also have some different opinions on such a practice. Second, even with such one exec being realized, I expect it needs an command option to tell what run it is . While that kind of options are , as Sam mentioned, treated with the help of the introduced dic-like data structure.

SamuelDegelia-NOAA · 2024-06-15T00:39:06Z

I updated this PR to now use the function get_all_keys instead of regex. Hopefully this is good to go now.

TingLei-NOAA

@SamuelDegelia-NOAA the current version looks working cleanly with much improved readability by using the dic-like structure. Thanks.

Reorganize rrfs-test/CMakeLists.txt to make it easier to add a ctest

84763fc

SamuelDegelia-NOAA requested review from ShunLiu-NOAA, delippi, guoqing-noaa and TingLei-NOAA June 14, 2024 16:39

SamuelDegelia-NOAA self-assigned this Jun 14, 2024

TingLei-NOAA reviewed Jun 14, 2024

View reviewed changes